-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add support for rendering mermaid diagrams #268
Conversation
1e3661a
to
5dc93f2
Compare
Wow that was fast! Looks very promising, however, I wasn't able to get this one working. I get a slide with the following info:
Additionally, this may break mmdc (not that bad maybe? It seems to work on github and vscode). To try this out this is what I ran:Presenterm mermaid testsTo test ran: cargo install --git https://github.com/mfontanini/presenterm/ --branch feat/mermaid-graphs yielding:
PR ExampleStats we care about pie title Pets adopted by volunteers
"Dogs" : 386
"Cats" : 85
"Rats" : 15
User Journeyjourney
title My working day
section Go to work
Make tea: 5: Me
Go upstairs: 3: Me
Do work: 1: Me, Cat
section Go home
Go downstairs: 5: Me
Sit down: 5: Me
Simple Flow chartflowchart TB
A --> C
A --> D
B --> C
B --> D
flow chart with style%%{init: {"flowchart": {"htmlLabels": false}} }%%
flowchart LR
markdown["`This **is** _Markdown_`"]
newLines["`Line1
Line 2
Line 3`"]
markdown --> newLines
Mind map!mindmap
root((mindmap))
Origins
Long history
::icon(fa fa-book)
Popularisation
British popular psychology author Tony Buzan
Research
On effectiveness<br/>and features
On Automatic creation
Uses
Creative techniques
Strategic planning
Argument mapping
Tools
Pen and paper
Mermaid
|
Are you sure you are running the right presenterm version? I know you're showing the cargo install command but the error you're showing is exactly what you'd get if you didn't install it.
Not sure I understand. What do you mean by this? |
I just pushed a commit. Now instead of "language Unknown does not support rendering" you should see "language Unknown("mermaid") does not support rendering". Can you re-run? |
Oops, Figured out I had a homebrew version of presenterm conflicting the cargo installed one. I was able to test successfully, but it doesnt always seem consistent:
And
These seem to be related to performance issues
EG: ➜ ~ mmdc -i presenterm_mermaid_test.md
No mermaid charts found in Markdown input PerformanceFrom poking around myself, seems like this is largely a mermaid side issue. Seems like their next release should improve performance some; however, looking at the code here I'm wondering if we're hitting a really bad mmdc bottleneck. It looks like the mmdc -i ... -o ... command for each mermaid chart. I think this makes it hit the worst part of mmdc (starting up a headless chromium instance) for each chart. |
This is not using a markdown file, it's writing each mermaid diagram snippet into a separate .mmd file so the
This seems related to when you're using markdown, which is not the case here so it won't improve anything here.
I hadn't seen how this worked... So in order to convert text into an image from the command line this spins up an entire browser... Software engineering has gone too far. Having said that, I can't reproduce the issue you have. I'm using the example presentation you pasted above and it works every time. You're saying sometimes it works and sometimes it doesn't? If you run |
I tried this out, and was able to render the "Stats we all care about"
😄 I agree. This approach is quite cursed - but I think it can be improved. Here
|
I also experimented a little with having some sort of background process that could help avoid starting and closing the render context (the browser). I tried out https://github.com/TomWright/mermaid-server but it doesn't seem to have incredible performance. Here's a demo of me calling it via a curl script and rendering directly to wezterm: mermaid-server-demo.movAs you can see, the performance isn't incredible. |
f9949a8
to
37d8f61
Compare
Yeah, images are already being cached in memory, it's just that the mermaid rendering isn't using it. I'll make a note to have them use it, this will improve the experience when developing a presentation.
And yes, this is something I considered when I created this PR. Looks like it will be a requirement here since this is incredibly slow. I'd suggest people go complain on this issue mermaid-js/mermaid#3650 as we're ultimately bound by their implementation. We can parallelize as much as we can but we'll never render charts faster than it takes to render a single one. I'm going to merge this as-is as the functionality is there. The 2 points above are QOL improvements but they're not blockers for having this in. I'll see if I can make up some time and work on them this week. |
This adds support for rendering mermaidjs diagrams by using the
+render
modifier. This requires the mermaid cli and accepts a few parameters in the theme and config file.In the theme:
mermaid.background
the background color passed to the CLI. Can betransparent
.mermaid.theme
the mermaid theme used/In the config file:
mermaid.scale
the scale passed to the mermaid CLI. Adjust if necessary if images look to small/large, this depends on your setup (e.g. terminal size and display resolution).The following presentation:
Gets rendered like this:
Fixes #266